AVRO-4325: [Trevni] Validate column-file header counts and lengths before allocating - #3921
Merged
RyanSkraba merged 2 commits intoAug 6, 2026
Merged
Conversation
The Trevni readers sized several allocations directly from values read from the file header/metadata without validating them against the input available. For a malformed, corrupted, or truncated file these counts/lengths could greatly exceed the bytes present, driving oversized allocations, or overflow to a negative size. Add a shared InputBuffer.checkLength/remaining helper that rejects a negative value and one that could not be backed by the bytes remaining, and apply it to: - ColumnFileReader.readHeader (column count) - ColumnDescriptor.ensureBlocksRead (block count) - InputBuffer.readBytes/readString (length-prefixed byte arrays) - ColumnValues.startBlock (compressed block size), which now also uses Math.addExact for the checksum size to guard against integer overflow. Reading a malformed file now fails fast with a clear IOException; valid files read unchanged.
There was a problem hiding this comment.
Pull request overview
Hardens Trevni Java readers against malformed/corrupted/truncated files by validating header/metadata-derived counts and lengths before using them to size allocations, so reads fail fast with IOException instead of attempting oversized/negative allocations.
Changes:
- Added
InputBuffer.remaining()andInputBuffer.checkLength(count, minBytesPerElement)and applied it to length-/count-prefixed reads (readString,readBytes). - Validated column count (
ColumnFileReader.readHeader) and block count (ColumnDescriptor.ensureBlocksRead) before allocating. - Added a regression test that corrupts the on-disk header column count to ensure fast rejection.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| lang/java/trevni/core/src/test/java/org/apache/trevni/TestColumnFile.java | Adds a regression test for rejecting an oversized header column count. |
| lang/java/trevni/core/src/main/java/org/apache/trevni/InputBuffer.java | Introduces shared “remaining bytes” + length/count validation and applies it to length-prefixed reads. |
| lang/java/trevni/core/src/main/java/org/apache/trevni/ColumnValues.java | Validates compressed block size before allocation and attempts to guard checksum-size addition. |
| lang/java/trevni/core/src/main/java/org/apache/trevni/ColumnFileReader.java | Validates header column count against remaining bytes before allocation. |
| lang/java/trevni/core/src/main/java/org/apache/trevni/ColumnDescriptor.java | Validates block count against remaining bytes before allocation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
97
to
99
| int end = in.checkLength(column.blocks[block].compressedSize, 1); | ||
| byte[] raw = new byte[Math.addExact(end, checksum.size())]; | ||
| in.readFully(raw); |
Member
Author
There was a problem hiding this comment.
Good catch on both points. startBlock now validates the combined compressedSize + checksum.size() length against the bytes remaining (computed in long to avoid overflow) and rejects a negative or overflowing size with an IOException, replacing Math.addExact (which could surface an unchecked ArithmeticException). A file truncated before the checksum is therefore rejected up front rather than reading past the remaining bytes. (d8ffcef)
| } | ||
|
|
||
| /** Byte offset of the little-endian 4-byte columnCount field in the header. */ | ||
| private static final int COLUMN_COUNT_OFFSET = 12; // MAGIC(4) + rowCount fixed64(8) |
Member
Author
There was a problem hiding this comment.
Done — the offset is now derived as ColumnFileWriter.MAGIC.length + Long.BYTES instead of the hard-coded 12, so it stays correct if the magic or fixed-width encoding sizes change. (d8ffcef)
…rive test offset ColumnValues.startBlock now validates the combined compressed-block-plus-checksum length against the bytes remaining (computed in long to avoid overflow) and rejects a negative or overflowing size with an IOException, instead of using Math.addExact (which could throw an unchecked ArithmeticException) and validating only the compressed size. The TestColumnFile column-count offset is now derived from ColumnFileWriter.MAGIC.length + Long.BYTES rather than a hard-coded 12.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (3)
lang/java/trevni/core/src/main/java/org/apache/trevni/ColumnValues.java:104
- The overflow error message for
end + checksumSizedoesn’t report the actual maximum allowed length, and it also contains an unnecessary string concatenation ("maximum " + "array size"). IncludingInteger.MAX_VALUEmakes failures easier to diagnose.
if (end > Integer.MAX_VALUE - checksumSize)
throw new IOException(
"Block size " + end + " plus checksum size " + checksumSize + " exceeds the maximum " + "array size");
lang/java/trevni/core/src/main/java/org/apache/trevni/ColumnValues.java:97
- The comment says the combined length is computed "in long" to avoid integer overflow, but the code avoids overflow via explicit
Integer.MAX_VALUE - checksumSizebounds checking (the addition itself is still done inint). This makes the comment misleading for future maintainers.
This issue also appears on line 102 of the same file.
// The block on disk is the compressed payload followed by the checksum
// bytes. Validate the combined length against the bytes remaining before
// allocating, computing in long to avoid integer overflow, so a malformed,
// corrupted, or truncated file fails fast with an IOException rather than an
// oversized/negative allocation or an unchecked ArithmeticException.
lang/java/trevni/core/src/main/java/org/apache/trevni/InputBuffer.java:343
checkLength(...)changes the failure mode for malformed/truncated inputs (e.g., negative or oversized lengths) to throwIOException, but there are no tests asserting these new rejection paths forreadString()/readBytes(). Adding a couple of negative tests would help prevent regressions of this security hardening.
public String readString() throws IOException {
int length = checkLength(readInt(), 1);
if (length <= (limit - pos)) { // in buffer
String result = utf8.decode(ByteBuffer.wrap(buf, pos, length)).toString();
pos += length;
RyanSkraba
pushed a commit
that referenced
this pull request
Aug 6, 2026
…fore allocating (#3921) * AVRO-4325: [Trevni] Validate header counts and lengths before allocating The Trevni readers sized several allocations directly from values read from the file header/metadata without validating them against the input available. For a malformed, corrupted, or truncated file these counts/lengths could greatly exceed the bytes present, driving oversized allocations, or overflow to a negative size. Add a shared InputBuffer.checkLength/remaining helper that rejects a negative value and one that could not be backed by the bytes remaining, and apply it to: - ColumnFileReader.readHeader (column count) - ColumnDescriptor.ensureBlocksRead (block count) - InputBuffer.readBytes/readString (length-prefixed byte arrays) - ColumnValues.startBlock (compressed block size), which now also uses Math.addExact for the checksum size to guard against integer overflow. Reading a malformed file now fails fast with a clear IOException; valid files read unchanged. * AVRO-4325: Address review: validate compressed size plus checksum; derive test offset ColumnValues.startBlock now validates the combined compressed-block-plus-checksum length against the bytes remaining (computed in long to avoid overflow) and rejects a negative or overflowing size with an IOException, instead of using Math.addExact (which could throw an unchecked ArithmeticException) and validating only the compressed size. The TestColumnFile column-count offset is now derived from ColumnFileWriter.MAGIC.length + Long.BYTES rather than a hard-coded 12.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
The Trevni readers sized several allocations directly from values read from the file header/metadata without validating them against the input actually available. For a malformed, corrupted, or truncated file these counts/lengths could greatly exceed the bytes present, driving oversized allocations, or overflow to a negative size.
This adds a shared
InputBuffer.remaining()/InputBuffer.checkLength(count, minBytesPerElement)helper that rejects a negative value and any value that could not be backed by the bytes remaining, and applies it to the reader paths that size allocations from header/metadata values:ColumnFileReader.readHeader— column countColumnDescriptor.ensureBlocksRead— block countInputBuffer.readBytes/readString— length-prefixed byte arraysColumnValues.startBlock— compressed block size; this path now also usesMath.addExactfor the+ checksum.size()addition to guard against integer overflow.Reading a malformed file now fails fast with a clear
IOException; valid files read unchanged.How was this patch tested?
TestColumnFile:oversizedColumnCountIsRejected— a file whose header column count is overwritten withInteger.MAX_VALUEnow fails fast instead of attempting a large allocation.trevni-coremodule test suite passes.JIRA